Skip to content

fix: remove unnecessary/unsafe type assertions in RawList and VirtualList#53

Merged
zombieJ merged 2 commits into
react-component:masterfrom
aojunhao123:fix/avoid-unnecessary-type-assertions
Jul 2, 2026
Merged

fix: remove unnecessary/unsafe type assertions in RawList and VirtualList#53
zombieJ merged 2 commits into
react-component:masterfrom
aojunhao123:fix/avoid-unnecessary-type-assertions

Conversation

@aojunhao123

@aojunhao123 aojunhao123 commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Replaced the React.forwardRef(X as any) as any double-cast used for RawListWithRef/VirtualListWithRef with the same typed-cast pattern already used by Listy/GroupHeader in this codebase, restoring prop type checking on those exports.
  • Removed the redundant as PositionScrollToConfig cast in useRawListScroll — control-flow narrowing already produces the correct type there (verified against both the root typescript@6.0.3 and father's internally-pinned typescript@5.4.2).
  • Left the item[rowKey] as React.Key casts in place — T is a fully generic type parameter, so a conditional/mapped type can't narrow T[keyof T] to React.Key for an abstract T (confirmed by attempting a KeysMatching<T, React.Key> restriction on RowKey<T>, which tsc rejects). This is a real boundary of the type system, not an oversight.
  • Left the config as number | ScrollConfig | null cast in VirtualList's scrollTo in place too. It initially looked removable under the root typescript@6.0.3, but pnpm compile failed: father resolves its own typescript@5.4.2 for declaration generation, and that version doesn't narrow the 'key' in config || 'groupKey' in config early-return pattern precisely enough to drop GroupScrollToConfig from the union, so it still needs the cast.

Test plan

  • npx tsc --noEmit -p tsconfig.json (typescript@6.0.3)
  • father's internal typescript@5.4.2 via direct tsc --noEmit -p tsconfig.json
  • pnpm compile (father build + declaration generation)
  • npx eslint src/ --ext .tsx,.ts
  • npx rc-test (4 suites, 40 tests, all passing)

…List

Two scroll-config casts were dead weight — control-flow narrowing already
produced the correct type. The forwardRef exports for RawList/VirtualList
used a double `any` cast that dropped all prop type checking; switched them
to the same typed-cast pattern already used by Listy/GroupHeader.
@vercel

vercel Bot commented Jul 1, 2026

Copy link
Copy Markdown

@aojunhao123 is attempting to deploy a commit to the React Component Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

本次改动将 RawListVirtualListforwardRef 导出及 scrollTo 配置相关的 as any 类型断言替换为携带泛型的精确类型签名,同时移除不再需要的类型导入,未涉及运行时逻辑变更。

Changes

类型精确化

Layer / File(s) Summary
RawList forwardRef 与 scrollTo 类型精确化
src/RawList/index.tsx, src/RawList/useRawListScroll.ts
RawListWithRefforwardRef 导出改为携带泛型的精确函数签名(返回 React.ReactElement),useRawListScroll 移除 PositionScrollToConfig 类型导入并直接从 config 解构 left/top
VirtualList forwardRef 与 scrollTo 类型精确化
src/VirtualList/index.tsx
移除 ScrollConfig 类型导入及 scrollTo 调用处的显式类型断言,VirtualListWithRefforwardRef 导出改为携带泛型的精确函数签名。

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • react-component/listy#46: 主 PR 精化的 forwardRef/refscrollTo 类型正是 #46 引入的原始列表吸顶功能所使用的同一组件与导出接口。
  • react-component/listy#47: 两者都改动了 src/RawList/useRawListScroll.ts 中的 scrollTo 逻辑,分别针对位置配置类型与吸顶分组逻辑。

Suggested reviewers: zombieJ

Poem

小兔子敲敲键盘,
把 any 都藏进洞里,
泛型签名闪闪亮,
scrollTo 也更清晰啦。
🐇✨ 跳过类型的荆棒林。

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed 标题准确概括了本次对 RawList 和 VirtualList 中不必要/不安全类型断言的移除。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens TypeScript typing around list scrolling and forwardRef exports by removing redundant/unsafe type assertions and replacing any-based forwardRef casts with the project’s established typed generic pattern.

Changes:

  • Removed unnecessary as assertions around scroll config handling in VirtualList and useRawListScroll.
  • Replaced React.forwardRef(X as any) as any with a typed generic forwardRef cast pattern for RawListWithRef and VirtualListWithRef.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/VirtualList/index.tsx Removes redundant scrollTo type assertion and replaces any-based forwardRef export with typed generic pattern.
src/RawList/useRawListScroll.ts Removes redundant cast/import by relying on control-flow narrowing for position scroll config.
src/RawList/index.tsx Replaces any-based forwardRef export with typed generic pattern consistent with the codebase.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves TypeScript type safety across the RawList and VirtualList components. It replaces loose 'any' type assertions on React.forwardRef with strongly-typed generic signatures. Additionally, it cleans up unused type imports and removes unnecessary type assertions when handling scroll configurations. I have no further feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

… 5.4.2

pnpm compile failed: father resolves its own typescript@5.4.2 for
declaration generation, independent of the root devDependency
(typescript@6.0.3). TS 5.4.2 doesn't narrow the compound
'key' in config || 'groupKey' in config early-return as precisely as 6.x,
so it still sees the full ListyScrollToConfig union at the scrollTo call
and rejects it without the cast.
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (26aff5f) to head (7a24e5e).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #53   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          211       211           
  Branches        62        63    +1     
=========================================
  Hits           211       211           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vercel

vercel Bot commented Jul 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
listy Error Error Jul 2, 2026 2:43am

@zombieJ zombieJ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zombieJ zombieJ merged commit f40d576 into react-component:master Jul 2, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants